fix ec validate input failing when policy has publicKey by robnester-rh · Pull Request #3134 · conforma/cli

robnester-rh · 2026-02-27T14:29:06Z

When a policy spec includes a publicKey field, ec validate input would fail with 'no check options or sig verifier configured' because the signature verifier isn't initialized for input validation scenarios.

Return the publicKey from the policy spec directly when SigVerifier is not initialized.

Ref: #1528
Ref: EC-1666

When a policy spec includes a publicKey field, ec validate input would fail with 'no check options or sig verifier configured' because the signature verifier isn't initialized for input validation scenarios. Return the publicKey from the policy spec directly when SigVerifier is not initialized. Ref: conforma#1528 Ref: EC-1666 Co-authored-by: Claude Code <noreply@anthropic.com> Signed-off-by: Rob Nester <rnester@redhat.com>

qodo-code-review · 2026-02-27T14:29:21Z

Review Summary by Qodo

Fix ec validate input with publicKey in policy spec

🐞 Bug fix

Walkthroughs

Description

• Fix ec validate input failing when policy spec includes publicKey field
• Return publicKey directly from policy spec when SigVerifier not initialized
• Add test case for publicKey in policy configuration
• Add acceptance test scenario for issue #1528

Diagram

flowchart LR
  A["Policy with publicKey"] -->|"SigVerifier not initialized"| B["PublicKeyPEM method"]
  B -->|"Previous: error"| C["Failed validation"]
  B -->|"Fixed: return publicKey"| D["Successful validation"]

File Changes

1. internal/policy/policy.go 🐞 Bug fix +5/-1

Return publicKey when SigVerifier not initialized

• Modified PublicKeyPEM() to return publicKey directly from policy spec when SigVerifier is not
 initialized
• Changed error return to []byte(p.PublicKey) instead of error for nil SigVerifier case
• Added explanatory comment referencing issue #1528

internal/policy/policy.go

2. internal/policy/policy_test.go 🧪 Tests +5/-3

Update test for publicKey without SigVerifier

• Updated test case name to reflect new behavior: "checkOpts is nil but publicKey in spec"
• Changed test expectation from error to successful publicKey return
• Fixed test data to use utils.TestPublicKeyJSON for proper JSON formatting
• Added comment explaining the fix for issue #1528

internal/policy/policy_test.go

3. acceptance/examples/happy_config_with_public_key.yaml 📝 Documentation +7/-0

Add example policy config with publicKey
• New example policy configuration file with embedded publicKey field
• Includes reference to happy-day-policy for testing
• Demonstrates valid policy structure for issue #1528 scenario
acceptance/examples/happy_config_with_public_key.yaml

View more (1)

4. features/validate_input.feature 🧪 Tests +24/-0

Add acceptance test for publicKey validation

• Added new acceptance test scenario for issue #1528
• Tests ec validate input with policy containing publicKey field
• Verifies successful validation with exit status 0
• Uses new happy-day-config-with-public-key repository

features/validate_input.feature

qodo-code-review · 2026-02-27T14:29:22Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📎 Requirement gaps (0)

1. PublicKeyPEM returns non-PEM ☑ 📘 Rule violation ✓ Correctness

Description

When SigVerifier is not initialized, PublicKeyPEM() returns p.PublicKey verbatim (which may be
a reference like k8s://..., not PEM). This contradicts the method’s name/contract and can confuse
callers or cause downstream failures if PEM is assumed.

Code

internal/policy/policy.go[R101-106]

+	// If SigVerifier is not initialized but we have PublicKey in the policy spec,
+	// return it directly. This handles scenarios like "ec validate input" where
+	// signature verification is not performed but the policy spec may contain a
+	// publicKey field (fixes issue #1528).
  if p.checkOpts == nil || p.checkOpts.SigVerifier == nil {
-		return nil, errors.New("no check options or sig verifier configured")
+		return []byte(p.PublicKey), nil

Evidence
The compliance checklist requires self-documenting identifiers; however, PublicKeyPEM() now
returns raw PublicKey bytes in some cases rather than PEM-encoded key material, making the API
misleading.
Rule 2: Generic: Meaningful Naming and Self-Documenting Code
internal/policy/policy.go[101-106]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`PublicKeyPEM()` can return `p.PublicKey` verbatim when `SigVerifier` is nil, which may not be PEM. This makes the method name/contract misleading and risks callers treating non-PEM values as PEM.
## Issue Context
This change was introduced to allow `ec validate input` to succeed when `policy.yaml` specifies `publicKey` without initializing signature verification.
## Fix Focus Areas
- internal/policy/policy.go[101-113]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

ⓘ The new review experience is currently in Beta. Learn more

codecov · 2026-02-27T14:48:45Z

Codecov Report

❌ Patch coverage is 87.50000% with 1 line in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/policy/policy.go	87.50%	1 Missing ⚠️

Flag	Coverage Δ
acceptance	`54.86% <75.00%> (-0.72%)`	⬇️
generative	`18.14% <0.00%> (-0.35%)`	⬇️
integration	`26.99% <0.00%> (-0.51%)`	⬇️
unit	`68.66% <87.50%> (+0.21%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
internal/policy/policy.go	`91.90% <87.50%> (-0.14%)`	⬇️

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

When SigVerifier was not initialized, PublicKeyPEM() returned the policy's PublicKey verbatim, which could be non-PEM (e.g. a key ref). Validate with pem.Decode and return an error when not valid PEM so callers do not treat non-PEM data as PEM. Ref: conforma#1528 Ref: EC-1666 Co-authored-by: Claude Code <noreply@anthropic.com> Signed-off-by: Rob Nester <rnester@redhat.com>

robnester-rh · 2026-03-04T13:59:52Z

/review

qodo-code-review · 2026-03-04T14:00:27Z

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 1528 - Partially compliant Compliant requirements: Add/adjust coverage so this regression is prevented going forward. Non-compliant requirements: Ensure `ec validate input` works with `publicKey` values like `k8s://openshift-pipelines/public-key` (i.e., non-PEM references present in the policy config). Requires further human verification: Confirm end-to-end that `ec validate input` succeeds when policy `publicKey` is a `k8s://...` reference (the reported failing case), not only when it is inline PEM. Validate that any callers of `PublicKeyPEM()` do not break if a non-PEM `publicKey` exists but signature verification is intentionally not initialized (input validation scenario).
⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review Behavioral Gap The new `PublicKeyPEM()` behavior returns the policy `PublicKey` only if it decodes as PEM when `SigVerifier` is not initialized; otherwise it errors. The original bug report includes `publicKey: k8s://...`, which is not PEM, so `ec validate input` may still fail for the reported scenario. Consider whether input validation should (a) ignore `publicKey` entirely when verification is not configured, or (b) return the raw reference without forcing PEM, depending on how downstream consumers use this method. // PublicKeyPEM returns the PublicKey in PEM format. When SigVerifier is not // initialized, the policy's PublicKey is returned only if it is already valid // PEM; otherwise an error is returned so callers do not treat non-PEM data as PEM. func (p policy) PublicKeyPEM() ([]byte, error) { // Public key is not involved when using keyless verification if p.Keyless() { return []byte{}, nil } // When SigVerifier is not initialized, return the policy's PublicKey only if // it is valid PEM (e.g. "ec validate input" with publicKey in policy.yaml). if p.checkOpts == nil \|\| p.checkOpts.SigVerifier == nil { if p.PublicKey == "" { return []byte{}, nil } if !isPEM([]byte(p.PublicKey)) { return nil, errors.New("public key is not in PEM format and signature verifier is not initialized") } return []byte(p.PublicKey), nil } API Contract* Introducing `isPEM()` as a gate changes the effective contract of `PublicKeyPEM()` when verification isn’t initialized. Re-check all call sites’ expectations: some may call `PublicKeyPEM()` merely to check presence/configuration and not to parse PEM, and will now receive an error for valid non-PEM key references. // isPEM reports whether data contains at least one valid PEM block. func isPEM(data []byte) bool { block, _ := pem.Decode(data) return block != nil } Test Coverage The added acceptance scenario uses an inline PEM `publicKey`, but the ticket’s failing case is a `k8s://...` reference. Consider adding a scenario that mirrors the real-world `k8s://` value (even if it’s not resolvable in tests) to ensure `validate input` doesn’t fail just because the field is present. # Test for issue #1528: ec validate input should work when policy has publicKey Scenario: valid policy URL with publicKey in policy config Given a git repository named "happy-day-config-with-public-key" with \| policy.yaml \| examples/happy_config_with_public_key.yaml \| Given a git repository named "happy-day-policy" with \| main.rego \| examples/happy_day.rego \| Given a pipeline definition file named "pipeline_definition.yaml" containing """ --- apiVersion: tekton.dev/v1 kind: Pipeline metadata: name: basic-build spec: tasks: - name: appstudio-init taskRef: name: init version: "0.1" """ When ec command is run with "validate input --file pipeline_definition.yaml --policy git::https://${GITHOST}/git/happy-day-config-with-public-key.git --output json" Then the exit status should be 0 Then the output should match the snapshot

github-actions bot added the size: M label Feb 27, 2026

robnester-rh requested a review from simonbaird March 3, 2026 16:12

qodo-code-review bot added the Review effort 3/5 label Mar 4, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix ec validate input failing when policy has publicKey#3134

fix ec validate input failing when policy has publicKey#3134
robnester-rh wants to merge 2 commits intoconforma:mainfrom
robnester-rh:EC-1666

robnester-rh commented Feb 27, 2026

Uh oh!

qodo-code-review bot commented Feb 27, 2026

Uh oh!

qodo-code-review bot commented Feb 27, 2026 •

edited

Loading

Uh oh!

codecov bot commented Feb 27, 2026 •

edited

Loading

Uh oh!

robnester-rh commented Mar 4, 2026

Uh oh!

qodo-code-review bot commented Mar 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

robnester-rh commented Feb 27, 2026

Uh oh!

qodo-code-review bot commented Feb 27, 2026

Review Summary by Qodo

Walkthroughs

File Changes

Uh oh!

qodo-code-review bot commented Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

codecov bot commented Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

robnester-rh commented Mar 4, 2026

Uh oh!

qodo-code-review bot commented Mar 4, 2026

PR Reviewer Guide 🔍

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

qodo-code-review bot commented Feb 27, 2026 •

edited

Loading

codecov bot commented Feb 27, 2026 •

edited

Loading